Skip to content

fix(windows): bigint probe identity in runtime preflight; release CC-Switch F3 WAL handles - #7986

Merged
lidge-jun merged 2 commits into
devfrom
fix/windows-lane-all-ino-ccswitch-20261030
Oct 30, 2026
Merged

lidge-jun merged 2 commits into
devfrom
fix/windows-lane-all-ino-ccswitch-20261030

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 30, 2026 •

Copy link
Copy Markdown
Owner

Summary

Fixes two Windows-only failures from manual Cross-platform CI run 41021347487 (lane=all, dev 3ea89653f).

  • windows 2/11, runtime preflight (product fix). NTFS file IDs can exceed 2^53, the largest integer a JS number holds exactly; the failing run reported ino 10696049116378316. Above 2^53, adjacent IDs compare equal as numbers. src/lib/bun-runtime-preflight.ts identified its own probe directory by number dev/ino, both in-process and in the child script. A replacement directory could therefore pass the "this is my probe" check and be removed. The check now uses lstatSync(path, { bigint: true }). The existing replacement-root test used the same number comparison and failed on CI, so it now compares bigints too. A new deterministic case injects an owned probe and a replacement whose IDs differ only above 2^53. Cleanup must leave the replacement in place. This case fails on the old source and passes on the new one.
  • windows 10/11, CC-Switch F3 (test fix). The F3 WAL test wrote rows with writer.prepare(...).run(...) and never finalized those statements. close() therefore did not release live-wal.db, -wal and -shm, and Windows refused to remove the temp tree (EBUSY, 15 s of retries). An isolated probe that did not call the product reproduced the EBUSY. After the statements were finalized, deletion succeeded. The importer closes its own descriptor on the refusal path, so the product needs no change. The test now uses writer.run(sql, ...params), which finalizes each statement. Its assertions are unchanged.

windows 4/11 of the same run (30 min timeout) was a slow runner. On that VM, setup steps took about 4× longer before any test ran (Setup Bun 26 s vs 5–8 s on the other ten shards, Install dependencies 64 s vs 12–18 s), and test durations rose broadly (median 1.36×, p75 2.6×). Neither repository change caused it, so this PR has no code change for it. The batched lane=all dispatch after merge will confirm.

Verification

  • bun test tests/lib/bun-runtime-preflight.test.ts before the src change: 15 pass / 1 fail (the new above-2^53 case). After: 59 pass / 0 fail across tests/lib/bun-runtime-preflight.test.ts, tests/service/service-runtime-preflight.test.ts, tests/service/service-desktop-runtime-preflight.test.ts and tests/codex-integration/codex-shim-runtime-preflight.test.ts (Windows 11, Bun 1.4.4).
  • bun test tests/providers/cc-switch-import.test.ts on Windows, before: 8 pass / 1 fail (F3 EBUSY at cleanup, 14.9 s). After: 9 pass / 0 fail, F3 49 ms.
  • bun run typecheck: 2,243 files, no errors.
  • bun run test:changed locally: first run hit its lane deadline (exit 124) on a machine shared with other test lanes; the rerun was stopped deliberately to free the shared host. Import-graph coverage comes from the hosted test changed job on the exact head (pass).
  • Fresh independent review at head 9b07b5c: PASS, including a scratch revert of the src that turned the new case red (15 pass / 1 fail) and a five-file focused run (68 pass / 0 fail).
  • NOT RUN: full bun run test locally, because other lanes are running Bun suites on the same machine. Hosted Windows coverage comes from the batched lane=all dispatch on dev after merge, since PR CI runs Ubuntu only.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (No user-facing behavior or doc contract changes.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. (The preflight change touches no credentials. It narrows what the probe cleanup may delete.)

NTFS file IDs exceed 2^53, where adjacent IDs are equal as JS numbers. The owned-probe check could then mistake a replacement directory for its own probe and remove it, and the replacement-root test failed on windows 2/11 of run 41021347487 with ino 10696049116378316.
Unfinalized prepare().run() statements keep the SQLite db, -wal and -shm open past close(), so Windows refused to remove the temp tree (EBUSY on windows 10/11 of run 41021347487).
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 30, 2026 11:33
@coderabbitai

coderabbitai Bot commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 37 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 0cc5a59 and 9b07b5c.

Files selected for processing: 3
  • src/lib/bun-runtime-preflight.ts
  • tests/lib/bun-runtime-preflight.test.ts
  • tests/providers/cc-switch-import.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Oct 30, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration (lane L1, cross-platform CI). Squash-merging into dev under the maintainer-integration exception in MAINTAINERS.md. This records the decision; it is not a self-approval.

Exact head: 9b07b5cc7a163b3e3571853f2724d3a93b14eb79
Exact-head CI: CI run 41048889756 (headSha 9b07b5c…, conclusion success): gates, test changed, storage policy, api usage, docker smoke, npm-global ubuntu, ci all success, plus Catalog async contracts, enforce-target and hygiene pass.
Review: a fresh independent reviewer, not the author, returned PASS at this head. It confirmed bigint dev/ino on both cleanup paths, found no incompatible fs injection in the service and shim callers, and turned the new regression case red against the old source. It also confirmed that Database.run finalizes its statement.
Union check vs current dev: this PR adds no files. None of the touched files has a file-size-baseline.json entry, and git merge-tree against current dev is clean.
Windows coverage: the batched lane=all dispatch on dev follows this merge.

@lidge-jun
lidge-jun merged commit ac0833b into dev Oct 30, 2026
30 checks passed
@lidge-jun
lidge-jun deleted the fix/windows-lane-all-ino-ccswitch-20261030 branch October 30, 2026 11:45
lidge-jun added a commit that referenced this pull request Oct 30, 2026
…#8002)

Like F3 (#7986), F1/F2 inserted with unfinalized prepare().run() statements, which keep the SQLite file open past close() on Windows and make the temp-tree cleanup fail with EBUSY. Use db.run(sql, ...params).

Co-authored-by: JUN <243035832+lidge-jun@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant